feat: add ready-for-dev issue and PR readiness gates - #509
Conversation
- .github/workflows/issue-readiness-check.yml: manages the ready-for-dev label on issues with type-specific criteria (bugs: reproducible command in Actual Behavior plus Acceptance Criteria checklist; enhancements: Desired Behavior plus Acceptance Criteria checklist). The readiness step emits JSON and always exits 0 so not-ready issues do not abort the workflow under set -euo pipefail; label add/remove and the feedback comment still run. Comments are upserted via a hidden marker so repeated runs update a single comment. - .github/workflows/pr-description-check.yml: pull_request_target gate requiring the Why/Summary/How to Test template sections and blocking PRs whose linked issues lack ready-for-dev (pre-rollout issues grandfathered). - .github/scripts/refresh_linked_pr_checks.py: re-runs the PR Description Check for open PRs linked to an issue when its ready-for-dev label changes, so the gate does not go stale. - Tests for all three scripts under tests/. Fixes #508 Co-authored-by: openhands <openhands@all-hands.dev>
neubig
changed the title
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Summary
This PR adds a ready-for-dev gate for issues (bug/enhancement) and a paired PR description gate that requires the template's ## Why, ## Summary, and ## How to Test sections, plus all linked issues to carry ready-for-dev (with a one-day grandfather window for issues opened before 2026-08-25). The architecture is sound: the pr-description-check workflow uses pull_request_target correctly (it checks out the base SHA and only runs trusted code from main), the issue-readiness scripts are pure functions of (body, labels), and the linked-gate refresh is a clean GraphQL lookup followed by a targeted workflow rerun. Tests are real — they import the production modules via importlib.util and exercise the readiness/extract/validate paths end-to-end, no mocks of the unit under test. The dependency footprint is zero. All 27 new tests pass locally.
The only suggestions worth mentioning are small: pull the duplicated acceptance-criteria check into a helper, simplify an over-engineered sort key, and fix the misleading "case-insensitive" wording on find_section. None are blockers.
Taste rating: 🟡 Acceptable
Solid implementation with clear tests and a pragmatic split into 3 Python scripts + 1 .mjs + 2 workflow files. The complexity is proportional to the feature (label-on/off automation + PR refresh + comment upsert) and individual pieces are each readable. Minor duplication and over-clever sort key keep it from a clean 🟢.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
The new code does not modify any runtime data plane — it adds CI validation only. Thepull_request_targetworkflow correctly checks out the base SHA so even fork PRs run only trusted code. No new dependencies; no permission escalations beyond minimalissues: write/actions: write/pull-requests: readscoped to the specific jobs that need them. The grandfathered-pre-rollout date (2026-08-25) is appropriately forward-looking.
VERDICT:
✅ Worth merging: Core logic is sound, all tests pass, minor improvements suggested in inline comments.
KEY INSIGHT:
The whole gate is intentionally a pure function of (body, labels) — check_issue_readiness.py and check_pr_description.py have no side effects beyond writing to stdout, which makes the entire suite unit-testable without fakes and gives the workflow authors the freedom to compose them any way they like.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation.
| "Add a `### Desired Behavior` section describing the behavior you want." | ||
| ) | ||
|
|
||
| acceptance = visible_text( |
There was a problem hiding this comment.
🟡 Suggestion: The acceptance-criteria block (lines 160-171) is a near-exact duplicate of the one in check_bug (135-146) — only the leading verb ("Add" vs. "Fill in") differs in two of the four messages. Pull it into a small helper (_validate_acceptance(sections, result) or similar) and call it from both check_bug and check_enhancement. Removes ~12 lines of copy-paste and makes any future change to the checklist rule a one-line edit.
| if not lines: | ||
| return False | ||
| # Created-at is sortable; pick the most recent run for this commit. | ||
| latest = sorted(lines, key=lambda item: " ".join(item[1:]))[-1][0] |
There was a problem hiding this comment.
🟡 Suggestion: key=lambda item: " ".join(item[1:]) is needlessly clever — the jq output is exactly two space-separated tokens (<id> <created_at>), so key=item[1] is equivalent and more obviously right. The current form makes readers wonder what extra fields could sneak in from the jq pipeline.
| return sections | ||
|
|
||
|
|
||
| def find_section(sections: dict[str, str], *labels: str) -> str: |
There was a problem hiding this comment.
🟡 Suggestion: The docstring says "by case-insensitive label", but extract_sections lowercases keys while the callers here happen to pass already-lowercased labels. Either make find_section actually case-insensitive (compare candidates against lowered keys) so callers can pass the canonical heading text, or strip the "case-insensitive" claim from the docstring. Right now the docstring is technically untrue.
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. The PR was approved with minor suggestions. View the conversation: https://app.all-hands.dev. |
4 participants
Why
The repository had no readiness gate: issues could be picked up before they contained enough detail to act on, and PRs could be opened against issues that were never triaged. This adds a repository-appropriate
ready-for-devworkflow and CI enforcement, adapted from the proven pattern inOpenHands/software-agent-sdkand tailored to extension contributions (skills, plugins, integrations, automations).Summary
.github/workflows/issue-readiness-check.yml: manages theready-for-devlabel with type-specific criteria. Bugs need an### Actual Behaviorsection with a reproducible command (uv run pytest,pytest,python,pip,npm run, ornode) plus an Acceptance Criteria checklist; enhancements need Desired Behavior plus an Acceptance Criteria checklist.set -euo pipefail: the readiness step emits JSON and always exits 0, so label removal and the feedback comment still run.post-readiness-comment.mjsupserts a single comment per issue via a hidden marker, and only fires on open/reopen or label transitions..github/workflows/pr-description-check.yml: apull_request_targetgate (trusted base-branch checkout, no fork code executed) requiring the## Why/## Summary/## How to Testtemplate sections and blocking PRs whose linked issues lackready-for-dev(issues predating the 2026-08-25 rollout are grandfathered).refresh_linked_pr_checks.py, so the gate never goes stale.tests/.Issue Number
Fixes #508
How to Test
uv run --group test pytest tests/- 768 passed, 12 skipped locally (32 new tests).python scripts/sync_extensions.py --checkandpython scripts/sync_openhands_sdk_skill.py --checkpass.check_issue_readiness.py --jsonunderset -euo pipefailfor a not-ready issue: exit 0, workflow processing continues, reasons emitted.check_pr_description.py --body-filefor a non-conforming body: exit 1 with::error::annotations.Video/Screenshots
N/A
Notes
main(issue events andpull_request_targetrun from the default branch), so they cannot be exercised end-to-end from this PR.READY_FOR_DEV_ROLLOUT_ISO = "2026-08-25") exempts pre-existing issues from the label requirement.This PR was created by an AI agent (OpenHands) on behalf of the user.
@neubig can click here to continue refining the PR